-
Notifications
You must be signed in to change notification settings - Fork 14.7k
KAFKA-19174 Gradle version upgrade 8 -->> 9 #19513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
A label of 'needs-attention' was automatically added to this PR in order to raise the |
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
Update: project compilation works as expected (when powered by Gradle 9) but binary release creation fails. |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
@dejan2609 any update? |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
@chia7712 |
@dejan2609 thanks! |
Pending: I will also explain second problem (specific to Gradle 9, that is). |
A label of 'needs-attention' was automatically added to this PR in order to raise the |
2ea0478
to
41e2d73
Compare
…it should be clearly visible now which submodules configurations had to be extended)
Update: an additional commit has been pushed (to polish some things). I also did some testing (locally, on my laptop):
Also, I revisited some previous shadow plugin related testing:
Testing procedure:
Test results:
|
@dejan2609 could you please take a look at my previous comment? #19513 (comment) |
Sure, I am creating POC to explain this case. |
APP_BASE_NAME=${0##*/} | ||
# Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036) | ||
APP_HOME=$( cd "${APP_HOME:-./}" > /dev/null && pwd -P ) || exit | ||
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excuse me, why to have those changes? Are those changed generated by gradle 9?
…for compatibility
@dejan2609 It would be helpful to understand the reasoning behind the changes in this PR, since introducing unnecessary modifications is usually not a good idea. That’s why I left some questions on parts where I couldn’t see the motivation—for example: #19513 (comment) Upgrading Gradle is an important task for Kafka, so I’d appreciate your help in clarifying the intent of this PR. Thanks! |
Yes, I understand the gravity of the situation. It goes without saying that these types of upgrades must be done with high caution. I have prior experience with upgrading Gradle major versions in Kafka (from Gradle 6 to 7 and then from 7 to 8) and they were complicated enough (but both of those upgrades pale in comparison to this one). I left answers to both comments above, but I will sum it up here also:
I opted to keep separate commits for every phase just for the sake of clarity (and that was a good decision, I guess). Options to proceed:
Note: not sure about the possibility of using option [3]: from where I see it 79f0415 (KAFKA-19591) is a must (either separately or as a part of upgrade); in more details: this is a minor bug on Gradle 8 level but could be a blocker for upgrade (if not included) on Gradle 9 level. Let me know what you think @chia7712 (I edited this comment few times, hence this note). |
@chia7712 Come to think of it: there is an option [4] also: I'll create a POC to narrow down options mentioned above 🙂 Stay tuned, I'll send an update here ASAP. |
Ok, I'll split this POC into multiple comments, starting from my local environment:
Overview of Related GitHub Gradle links for (the thing is that Gradle decided to change the path for
🪧 |
Now that we have an environment in place, we need to do the following things:
|
@chia7712 you can review this again. To sum it up (and please pardon me for such a long comments above, these things are quite tricky to catch and hence I wanted to provide you as much details as possible):
Where to proceed from here:
Note: maybe it would be a good idea to describe this corner case (for Gradle wrapper generation, that is) in README.md file. Signing off, will get back in 3 or 4 days 🥾 ⛰️ 👋 |
Hi @chia7712 ! I did some cleanup (resolved old/outdated conversations). You probably have a lot of things in your schedule, but please try to squeeze this in somehow. TLDR version for you:
Waiting for your input on this. |
@dejan2609 I open the https://issues.apache.org/jira/browse/KAFKA-19750 to describe the root cause of |
Got it. Should I add a commit that resolves that issue here or you want to solve it separately ? |
What do you think of this approache? Is it a viable replacement for the "distribution" module? If yes, it will be great this PR include the approach. If not, we could have a separate PR for that to fix the warnings for other active branches. |
Aha, now I see it clearly 💡 Allow me some time to test this idea, will update you asap. |
…t need this for a Gradle upgrade)
I may have some good news for you @chia7712 ! So, I just scrapped a skeleton POC solution #20625 with just two things:
Results look encouraging: Action points for me would be to reorganize all remaining commits into a new solution (that keeps If I may suggest something (my 2 cents on this): we could opt to solve KAFKA-19750 separately (i.e. on a Gradle 8 level). All-in-all: it is safe to say that things are wrapping up. |
You are right. @brandboat Do you have free time to file a patch for it first? |
Many thanks to the help @chia7712, @dejan2609. Here is the pr link: #20628 |
Related JIRA ticket: https://issues.apache.org/jira/browse/KAFKA-19174
List of changes:
9.0.09.1.09.0.28.3.9changes
Related links:
Notes: